feat(sdk): add Options.DebugLogger and WithDebugLogger option#66
feat(sdk): add Options.DebugLogger and WithDebugLogger option#66ezynda3 wants to merge 1 commit into
Conversation
Today the SDK exposes a DebugLogger interface (pkg/kit/types.go) but no public path to install one — the only consumer of the field is the unexported kit.AgentConfig.toInternal() method, which itself is not reachable from outside the package. As a result, embedders that want to forward Kit's low-level engine + MCP tool plumbing debug output into their own logging system (slog, zap, charm/log, an in-app TUI panel, etc.) have no option but the on/off Debug bool, which always installs the built-in SimpleDebugLogger / BufferedDebugLogger. This change closes that gap on the supported Options / functional-option construction path: - pkg/kit/kit.go: add Options.DebugLogger DebugLogger. When non-nil it is used directly and the Debug bool is ignored; the supplied logger's IsDebugEnabled() controls whether downstream code emits messages. - pkg/kit/options.go: add WithDebugLogger(l DebugLogger) Option. - internal/kitsetup/setup.go: add AgentSetupOptions.DebugLogger and switch SetupAgent's logger selection so the caller-supplied logger wins unconditionally; otherwise the existing Debug + UseBufferedLogger branch picks the built-in implementation. No behaviour change when DebugLogger is nil. - pkg/kit/kit.go: wire opts.DebugLogger into setupOpts so the New() path threads it through. - pkg/kit/viper_isolation_test.go: add TestWithDebugLoggerPlumbing and TestWithDebugLoggerNilClears covering the option-to-field contract and later-options-override semantics consistent with the other With* helpers. - pkg/kit/README.md: list WithDebugLogger in the helper inventory. Notes: - kit.DebugLogger and tools.DebugLogger are structurally identical (LogDebug(string) / IsDebugEnabled() bool), so the SDK value flows into the internal field without a conversion. - This is purely additive on the SDK surface and does not touch kit.AgentConfig — that field already carried a DebugLogger, but the AgentConfig path is unreachable from outside the package today.
|
Connected to Huly®: KIT-67 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds a ChangesWithDebugLogger Custom Debug Logger
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Closing in favour of #67, which bundles this additive |
Summary
The SDK exposes a
DebugLoggerinterface (pkg/kit/types.go) but no public path to install one. The only consumer of the field is the unexportedkit.AgentConfig.toInternal()method, which itself is not reachable from outside the package. Embedders that want to forward Kit's low-level engine + MCP debug output into their own logging system (slog, zap, charm/log, an in-app TUI panel, etc.) currently have no option but the on/offDebug bool, which always installs the built-inSimpleDebugLogger/BufferedDebugLogger.This PR closes that gap on the supported
Options/ functional-option construction path.Changes
pkg/kit/kit.go: addOptions.DebugLogger DebugLogger. When non-nil it is used directly and theDebugbool is ignored; the supplied logger'sIsDebugEnabled()controls whether downstream code emits messages.pkg/kit/options.go: addWithDebugLogger(l DebugLogger) Option.internal/kitsetup/setup.go: addAgentSetupOptions.DebugLoggerand switchSetupAgent's logger selection so the caller-supplied logger wins unconditionally; otherwise the existingDebug+UseBufferedLoggerbranch picks the built-in implementation. No behaviour change whenDebugLoggeris nil.pkg/kit/kit.go: wireopts.DebugLoggerintosetupOptsso theNew()path threads it through.pkg/kit/viper_isolation_test.go: addTestWithDebugLoggerPlumbingandTestWithDebugLoggerNilClearscovering the option-to-field contract and later-options-override semantics consistent with the otherWith*helpers.pkg/kit/README.md: listWithDebugLoggerin the helper inventory.Usage
Notes
kit.DebugLoggerandtools.DebugLoggerare structurally identical (LogDebug(string)/IsDebugEnabled() bool), so the SDK value flows into the internal field without a conversion.kit.AgentConfig— that field already carried aDebugLogger, but theAgentConfigpath is unreachable from outside the package today.kit.AgentConfigentirely; this PR is intentionally scoped to the "keepDebugLogger, add the missing wiring" option only. Removal ofAgentConfig(if desired) belongs in a separate change.Verification
go build ./pkg/... ./internal/... ./cmd/...— cleango vet ./...— clean (modulo the pre-existingexamples/extensionsYaegi source not being amainpackage)go test -race ./pkg/kit/... ./internal/kitsetup/... ./internal/agent/... ./internal/tools/...— passingSummary by CodeRabbit
Release Notes
WithDebugLoggeroption to supply custom debug logger implementations, allowing users to route and control debug output